Skip to content

Conversation

@pranavjain97
Copy link
Contributor

No description provided.

@pranavjain97 pranavjain97 changed the base branch from master to WP-5232-mpcv2-signing-custom-fns July 9, 2025 22:08
Base automatically changed from WP-5232-mpcv2-signing-custom-fns to master July 10, 2025 16:23
@pranavjain97 pranavjain97 force-pushed the WP-5238-sign-send-txrequest branch from 1bf290e to 505c288 Compare July 10, 2025 17:50
@pranavjain97 pranavjain97 changed the title Wp 5238 sign send txrequest feat(wp): api to sign and send a txrequest Jul 10, 2025
@pranavjain97 pranavjain97 force-pushed the WP-5238-sign-send-txrequest branch from 505c288 to 3c11b17 Compare July 10, 2025 19:40
@pranavjain97
Copy link
Contributor Author

pranavjain97 commented Jul 10, 2025

@claude review the PR

@pranavjain97
Copy link
Contributor Author

@claude review this pr

@pranavjain97 pranavjain97 requested a review from Copilot July 10, 2025 19:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a new endpoint to sign and send MPC transaction requests and integrates it into the API spec, router, handler, tests, and OpenAPI JSON.

  • Introduces handleSignAndSendTxRequest handler and wires it into MasterApiSpec and router
  • Defines request/response types (SignMpcRequest / SignMpcResponse) and adds tests for ECDSA and EdDSA flows
  • Updates masterBitgoExpress.json to include the new /txrequest/{txRequestId}/signAndSend path

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/api/master/routers/masterApiSpec.ts Added new route spec and registration for signAndSend
src/api/master/handlers/handleSignAndSendTxRequest.ts Implemented MPC sign-and-send handler logic
src/tests/api/master/signAndSendTxRequest.test.ts Added tests covering successful and pending flows
masterBitgoExpress.json Updated OpenAPI spec with new path (and removed passphrase parameter)
Comments suppressed due to low confidence (5)

masterBitgoExpress.json:258

  • The walletPassphrase property was removed from the consolidate request schema here but likely unintentionally. Reintroduce or confirm removal if intended.
                  },

src/api/master/routers/masterApiSpec.ts:228

  • Using t.any for the 200 response is too permissive. Define a specific codec that matches the expected response properties (e.g., txid, tx, optional pendingApproval, etc.).
  200: t.any,

masterBitgoExpress.json:634

  • The OpenAPI spec for the signAndSend endpoint has an empty schema for the 200 response—please specify the returned object structure to aid consumers.
                "schema": {}

src/api/master/routers/masterApiSpec.ts:222

  • [nitpick] SignMpcRequest is generic; consider renaming to SignAndSendTxRequestBody for clarity and consistency with the endpoint.
const SignMpcRequest = {

src/tests/api/master/signAndSendTxRequest.test.ts:22

  • [nitpick] Tests cover successful and pending cases but lack negative scenarios (e.g., wallet not found, wrong wallet type, or mismatched commonKeychain). Consider adding those to improve coverage.
describe('POST /api/:coin/wallet/:walletId/txrequest/:txRequestId/signAndSend', () => {

@pranavjain97 pranavjain97 force-pushed the WP-5238-sign-send-txrequest branch from 3c11b17 to 9fe64d7 Compare July 10, 2025 19:50
@pranavjain97 pranavjain97 changed the title feat(wp): api to sign and send a txrequest feat(mbe): api to sign and send a txrequest Jul 10, 2025
@pranavjain97 pranavjain97 enabled auto-merge July 10, 2025 22:00
}

if (wallet.type() !== 'cold' || wallet.subType() !== 'onPrem') {
throw new Error('Wallet is not an on-prem wallet');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Advanced Wallet

@pranavjain97 pranavjain97 merged commit acb3608 into master Jul 11, 2025
3 checks passed
@pranavjain97 pranavjain97 deleted the WP-5238-sign-send-txrequest branch July 11, 2025 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants